-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix flaky ssh integration tests #1587
fix flaky ssh integration tests #1587
Conversation
@billbsing after running the integration tests a few times, I think they're still flaky. I ran into this in the Parity integration tests a while back, see #1447. I think the best option is just to mark it as flaky like I did in #1447. Let me know if you disagree or if you don't want/have time to implement and I can pick it up. Thanks! |
I have added xfail on a ValueError around each test method using a post. I'm ok to try and find a better solution, if this does not work, or just cancel this PR and continue skipping the flaky tests for the time being. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@billbsing I don't think we need the time.sleep(1)
if we have the xfail
s. I also think have only seen it fail on the remove_filter
, remove_filter_deprecated
, shh_post
, and shh_post_deprecated
, so I'd vote to only keep the xfail
decorators on those tests until we start seeing others fail. I could be convinced otherwise, however, if you have a strong opinion.
On a higher level, it's strange that they just started failing, and that I've only seen them fail on geth 1.7. This fix is definitely treating the symptom rather than the cause and I haven't had time to dig into why they are suddenly failing periodically. If you have ideas you want to explore, feel free!
@@ -219,6 +236,10 @@ def test_shh_async_filter_deprecated(self, web3: "Web3") -> None: | |||
|
|||
watcher.stop() | |||
|
|||
# Sometimes the post fails because PoW is too low. | |||
# We don't care if an error or a True response comes back, | |||
# we only care that we're interfacing correctly with Parity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# we only care that we're interfacing correctly with Parity | |
# we only care that we're interfacing correctly with Geth |
@@ -246,6 +267,10 @@ def test_shh_remove_filter_deprecated(self, web3: "Web3") -> None: | |||
except BaseException: | |||
assert True | |||
|
|||
# Sometimes the post fails because PoW is too low. | |||
# We don't care if an error or a True response comes back, | |||
# we only care that we're interfacing correctly with Parity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# we only care that we're interfacing correctly with Parity | |
# we only care that we're interfacing correctly with Geth |
@@ -358,6 +383,10 @@ def test_shh_symmetric_key_pair_from_password(self, web3: "Web3") -> None: | |||
# | |||
# shh_post | |||
# | |||
# Sometimes the post fails because PoW is too low. | |||
# We don't care if an error or a True response comes back, | |||
# we only care that we're interfacing correctly with Parity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# we only care that we're interfacing correctly with Parity | |
# we only care that we're interfacing correctly with Geth |
|
||
# Sometimes the post fails because PoW is too low. | ||
# We don't care if an error or a True response comes back, | ||
# we only care that we're interfacing correctly with Parity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# we only care that we're interfacing correctly with Parity | |
# we only care that we're interfacing correctly with Geth |
This PR will not resolve the flaky shh module errors. |
What was wrong?
Related to Issue #1580
How was it fixed?
Checked that each
ssh.post
is followed by atime.sleep(1)